One blocking correctness issue:

apply_persona_snapshot now clears record.model for every linked definition whose model is blank. But record.model is also the persisted per-instance model set by update_managed_agent (commands/agent_models.rs:807-809), and the edit flow explicitly supports a “deliberate local model” (AgentInstanceEditDialog.tsx:622-687). There is no provenance bit here that distinguishes stale materialized inherited bytes from an intentional instance override.

Repro at the data-model level: linked definition has model = None, user edits that instance to model = Some("gpt-4o"), then restarts it. This assignment resets the model to None, so the restart silently replaces the user’s explicit choice with the global default. It also contradicts the existing fallback contract/test in persona_events/tests.rs:515-529.

Please preserve explicit instance models or add provenance that lets this path clear only inherited/materialized values. The regression test should cover an explicit per-instance model surviving restart; the new hash-convergence test only models the stale case and therefore cannot catch this loss.

Unified findings from two independent review passes (full-file reads at head 89dbbc7). Both passes converged on the same two blockers, so confidence is high on both. Inline comments below have the details; summary here.

Blocking

1. The primary Stop action now restarts running local agents instead of stopping them — the new restart param on handleAgentPrimaryAction gets the DOM click event as its first argument when the same function is installed directly as the button onClick.
2. record.model = persona.model.clone() on every snapshot apply can't distinguish stale materialized inheritance from an explicit per-instance model pick, so explicit picks (update_managed_agent, ModelPicker's persist path) get silently wiped on the next restart — and the restart badge never trips because spawn_config_hash runs the same prospective snapshot.

Non-blocking

• persona_snapshot_with_agent_config_fallback still computes snapshot.model, which is now dead — only snapshot.provider is consumed.
• Nit: the Restart quick action renders for stopped local agents too, where it duplicates Spawn/Respawn; gating on isManagedAgentActive would keep the surface cleaner.
• Nit: handleAgentPrimaryAction lost its useCallback and handleAgentRestart is a fresh arrow per render — harmless, but inconsistent with the surrounding handlers.

What's solid: respawnManagedAgentWithRules correctly awaits stop-before-start with a focused ordering test, and inherited_persona_model_converges_after_one_restart pins exactly the reported bug (badge-neutral one-restart convergence). The seam choice (apply_persona_snapshot) is the right place once model provenance is handled safely.

Independent pass at head 89dbbc74f (full-file reads of the snapshot seam + profile wiring). I confirm both existing blockers and add supporting evidence plus three smaller findings.

Blocker 1 confirmed — Stop now restarts (event-as-arg). handleAgentPrimaryAction(restart = false) is installed directly as onClick (UserProfilePanelSections.tsx:707), so React passes the click SyntheticEvent as the first argument. Truthy event → restart && backend.type === "local" is true for every local agent → clicking Stop on a running local agent silently stops and restarts it. The prop type () => void hides this from typecheck. The new node test only exercises respawnManagedAgentWithRules in isolation, so it can't catch the wiring bug.

Blocker 2 confirmed — explicit instance model destroyed, with a concrete UI repro. record.model = persona.model.clone() can't distinguish stale materialized bytes from a deliberate per-instance override. Worst-case flow is one the UI itself recommends: ModelPicker's non-live path persists an explicit pick via updateManagedAgent and then sets needsRestart (ModelPicker.tsx:156-163) — the restart it asks for is exactly what now wipes the pick (blank-definition persona → record.model = None → global default). The user follows the hint and loses their selection.

Supporting evidence that this forks a documented contract: persona_field_with_record_fallback (persona_events.rs:388-389) is explicitly the "single source of truth … so the three paths cannot drift" across snapshot-apply, build_deploy_payload, and resolve_effective_prompt_model_provider. This change forks apply away from the other two: the config surface (agent_config.rs:68) still shows the record's explicit model as effective while a restart now discards it. The existing contract test (fallback_preserves_record_values_when_persona_blank) still passes only because it tests the helper, not the apply — the contract is bypassed rather than updated.

Non-blocking:

• apply_persona_snapshot still computes snapshot.model via the fallback helper and then ignores it — dead computation that actively misleads readers about which rule applies.
• The Restart quick action renders for stopped local agents too, where it's just a duplicate Start; consider gating on isManagedAgentActive (or at least labeling accordingly).
• Desktop Smoke E2E (4) red is video-attachment.spec.ts:927 — unrelated to this diff (flake/pre-existing); still needs a green rerun before merge.

Direction thought (non-binding): the stale bytes exist because create-time materializes the then-current global default into record.model when the definition is blank. Fixing provenance at write time (don't persist inherited/global-derived values onto the record, or store a provenance bit) keeps the fallback contract intact and fixes the reported bug without making restart destructive.

Concur with changes-requested. Minimalness 8/10, Elegance 6/10 (contract fork + dead value), Correctness 5/10 (two user-visible regressions: Stop doesn't stop; explicit model picks lost on restart).

Re-review at e9603ccecc869440c77c325f93b3ef1087e930ef: both blockers remain, so my changes-requested stands.

1. Stop still restarts. handleAgentPrimaryAction(restart = false) is still passed directly through to the button onClick. React supplies the click event as argument 1, making restart truthy, so a running local agent takes the stop-then-start branch. Please split this into zero-argument primary/restart callbacks (or wrap the primary call) and cover the wiring, not only the helper.

2. Explicit linked-instance model choices are still discarded. The new resolver change does not preserve the override; it now explicitly ignores record.model whenever the linked definition exists (definition → global). ModelPicker still persists non-live choices to record.model and says “restart to apply,” so that UI path now writes a value spawn/readiness deliberately never consume. This makes the original regression immediate/structural rather than fixing it. The snapshot assignment still clears the record on restart as well.

The model fix needs either provenance to distinguish explicit instance values from stale inherited snapshots, or removal/gating of the unsupported per-instance edit path with coherent UI/config semantics. Please add the missing regression: choose an explicit model for a definition-linked instance, restart, and verify the chosen model is what spawn resolves. The current tests only rename stale bytes and codify ignoring them.

Re-reviewed at new head e9603ccec (delta vs 89dbbc74f is only global_config/mod.rs + its tests; persona_events.rs and the React wiring are byte-identical). Verdict: still changes-requested — one blocker untouched, the other turned into a semantics question that needs an explicit call.

1. Stop-restarts blocker is unaddressed. handleAgentPrimaryAction = async (restart = false) (UserProfilePanel.tsx:455) is still installed directly as the primary-action onClick, so the click SyntheticEvent arrives as restart. Clicking Stop on a running local agent still stops-then-starts it and toasts "Restarted". Mechanical fix: two zero-arg callbacks, or wrap the prop (() => void handleAgentPrimaryAction()), plus a wiring-level test.

2. The resolver pivot is coherent — but it orphans the per-instance model lever. New resolve_effective_model_provider: linked instances resolve model definition → global, record ignored. That genuinely fixes the stale-bytes bug and spawn/readiness/hash now agree; the new tests codify it cleanly. But record.model didn't stop existing:

• update_managed_agent still persists an instance model, and AgentInstanceEditDialog still offers it — its submit comment literally says "a deliberate local model still wins" (personaRuntimeModel.ts:197), which is now false for linked agents. The lever becomes a silent no-op.
• resolve_effective_prompt_model_provider (config surface, agent_config.rs:68) still uses record-fallback, so until the first restart wipes the bytes, the UI displays a model the spawn won't run — display/spawn drift within one head.
• One correction to my round-1 evidence: ModelPicker.tsx is exported but not mounted anywhere in this tree (grep: no importers), so the needs-restart-hint flow I cited isn't live — the Edit Agent dialog is the real path.

If "linked instances have no per-instance model" is intended product semantics, fine as the launch-narrow shape — but then this same PR must gate/disable the instance-model field for linked agents and align the config-surface resolver, otherwise we ship UI that lies. If per-instance overrides should survive, it's the provenance route instead. That's a product call — flagging for Tyler.

Carried nits (still present): apply_persona_snapshot computes snapshot.model via the fallback helper then discards it; Restart quick action renders for stopped agents (duplicate Start).

CI at e9603ccec: red — Desktop aggregate + Smoke E2E (4) (video-attachment.spec.ts:927, unrelated to this diff; still needs a green rerun at final head).

Ratings at this head: Minimalness 8/10, Elegance 7/10 (resolver is cleaner; dead fallback computation and orphaned lever remain), Correctness 5/10 (Stop still doesn't stop; display/spawn drift + no-op edit lever). Not ready.